src: cosmetic: simplify an invalid configuration check#10626
src: cosmetic: simplify an invalid configuration check#10626lyakh wants to merge 2 commits intothesofproject:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Simplifies the invalid configuration check in src_params_general() by directly validating cd->param.total before computing delay_lines_size.
Changes:
- Reorders validation to check
cd->param.total == 0before calculatingdelay_lines_size. - Updates the error message emitted on invalid configuration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The condition delay_lines_size == 0 in src_params_general() can trigger if cd->param.total == 0 or if cd->param.total == -1. However, the latter is supposedly invalid and should be checked in a more generic non-negativity test, so here it suffices to just check cd->param.total != 0 before delay_lines_size is calculated. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
If the delay line size hasn't changed no need to re-allocate the buffer. Also use size_t for some byte-size variables and structure members. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
|
@lrudyX AFAICS QB failures are unrelated: on MTL 2 HDA tests failed and none of them contains SRC components, while this PR only touches SRC. And the single PTL failure is our beloved "random HDA DMA" test. Could you confirm? |
|
Yes, QB is full of failures right now. |
Passing now, thanks @lrudyX |
| int filter_length; | ||
| int blk_in; | ||
| int blk_out; | ||
| size_t subfilter_length; |
There was a problem hiding this comment.
These are not sizes in bytes. Also in some architectures size_t is 64 bits. I prefer just int for a generic integer number. Also unsigned int would be better than size_t. But you know that I don't like it either. Just unnecessary code acrobatics to use a restricted range integer type. It's not improving code efficiency.
singalsu
left a comment
singalsu
left a comment
There was a problem hiding this comment.
Please drop changes to structs in src_common.h unless you find a size in bytes. I don't think there is but I didn't do an extensive check. Try ask Copilot to generate doxygen comments for the structs. It would help to understand them since the member names aren't always good.
I may use quite freely "size" or "length" for something that is not bytes, and sometimes it can be. E.g. "FFT size" is common in textbooks and it's about number of unit delays or samples.
The condition delay_lines_size == 0 in src_params_general() can trigger if cd->param.total == 0 or if cd->param.total == -1. However, the latter is supposedly invalid and should be checked in a more generic non-negativity test, so here it suffices to just check cd->param.total != 0 before delay_lines_size is calculated.